Skip to content

Comments

Recipe for getting local constraints#1014

Merged
Joao-Dionisio merged 10 commits intomasterfrom
getLocalConss
Jul 26, 2025
Merged

Recipe for getting local constraints#1014
Joao-Dionisio merged 10 commits intomasterfrom
getLocalConss

Conversation

@Joao-Dionisio
Copy link
Member

SCIP only allows one to get the constraints added to a given node. It is now possible to get all constraints valid in the focus node, excluding the original ones. Maybe we can also add a getLocalConsNode, or even an optional argument defaulting to the current node.

@Joao-Dionisio Joao-Dionisio requested a review from Copilot July 1, 2025 00:04

This comment was marked as outdated.

@Joao-Dionisio
Copy link
Member Author

Everything seems fine, will merge in a couple days, if everyone's alright with it.

@DominikKamp
Copy link
Contributor

Better iterate through the constraint handlers and get the active constraints using https://scipopt.org/doc-9.2.2/html/group__PublicConshdlrMethods.php#gae573c332a19e69c45163fadfeab3948b and https://scipopt.org/doc-9.2.2/html/group__PublicConshdlrMethods.php#gad610bfb6d34ee998835d8b9bbbb9cf8c and exclude original ones manually.

@Joao-Dionisio
Copy link
Member Author

Why is it better? You mean that some constraints are discarded for whatever reason, but they would appear here?
I'm just a bit fearful about returning transformed constraints.

@DominikKamp
Copy link
Contributor

It should also be checked that the constraints are enabled, but I rather wonder about the performance impact of walking back to the root for each node. If local constraints are added for every node, this might be okay though. However, I am not sure about the usecase of this set of constraints.

@Joao-Dionisio
Copy link
Member Author

Joao-Dionisio commented Jul 1, 2025

@DominikKamp mostly for debugging. But it seems useful to be able to get the constraints you added after starting to solve. Especially since users don't have an easy way to get the constraints that make up the local problem.
Initially, I thought about returning the local constraints plus the root constraints, but it would be less ergonomic for the user.

I guess interfacing SCIPwriteMIP makes sense, as well. Also, it's a bit ugly that both SCIPwriteLp and SCIPwriteLP exist.

Regarding performance, it doesn't seem that bad, plus it's a recipe.

@DominikKamp
Copy link
Contributor

But global constraints are also local constraints so the method name is actually misleading.

@Joao-Dionisio
Copy link
Member Author

What would you name it? I can also return all the local constraints (including original ones), but if users are interested in the non-original ones, they would need to dig through the array. One option would actually be to return a two-dimensional array instead. First entry with original constraints, second entry with non-original constraints.

@DominikKamp
Copy link
Contributor

DominikKamp commented Jul 1, 2025

Since you prepend the added constraints, you could also just include the root constraints and additionally provide their number, or even better return an array with all the lengths.

@Joao-Dionisio
Copy link
Member Author

Joao-Dionisio commented Jul 4, 2025

See how you like it now, @DominikKamp. It might be better if we just return [[nglobal_conss, global_conss], [nadded_cons, added_conss]]. I don't know if this was what you were suggesting in the first place.

EDIT: but then retrieving the constraints is very annoying, would need plenty of indices. I'm happy with this for now, since it's a recipe no need for perfection.

@DominikKamp
Copy link
Contributor

Actually I meant just two lists, one for all enabled constraints added on the whole path in sequence, another one with the number of constraints for each node on the path. Note, that all constraints disabled in the current node need to be excluded manually. Furthermore, you might want to skip the nodes before the effective root.

@Joao-Dionisio
Copy link
Member Author

If we're going to discriminate the number of constraints by node, then I'd favor a dictionary with node numbers as keys, otherwise, it seems like a mess.

The constraints are disabled, but still valid, no? Why would it matter if they're there or not? I'd rather have them, and then people check if they're active manually, otherwise it seems like they disappeared.

The effective root is a term I'm not familiar with, what do you mean? In theory, we could just check between nodes A and its ancestor B, but I'm assuming that B is always the actual root.

@DominikKamp
Copy link
Contributor

DominikKamp commented Jul 4, 2025

Fine with a dictionary for the numbers of constraints (though it might make it more difficult to access the number of global constraints).

Disabled constraints are technically not part of the local description but should be valid, just thought that it is the purpose of this function to provide the current set of local constraints.

By default, if all but one child of the current effective root is cut off, the single remaining child becomes the next effective root and all its bound and constraint set changes are globalized, but you actually need to go to the original root to get all global constraints.

@Joao-Dionisio
Copy link
Member Author

After digesting a bit, I'm fine with the way it is now. The purpose is to facilitate debugging, and it's also a recipe. If any users end up using it and have suggestions, please feel free to share.

Thanks for the input, Dominik!

@Joao-Dionisio Joao-Dionisio marked this pull request as draft July 26, 2025 11:05
@Joao-Dionisio Joao-Dionisio requested a review from Copilot July 26, 2025 11:05
@Joao-Dionisio Joao-Dionisio marked this pull request as ready for review July 26, 2025 11:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a recipe for retrieving local constraints from SCIP nodes. The main purpose is to provide a convenient way to access all constraints that are valid in the current focus node, excluding the original global constraints.

Key Changes:

  • Adds a new recipe module getLocalConss.py with functions to retrieve local constraints and their counts
  • Includes comprehensive test coverage for the new functionality
  • Adds a writeMIP method to output MIP relaxations

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/pyscipopt/recipes/getLocalConss.py New recipe module implementing the core functionality for retrieving local constraints
tests/test_recipe_getLocalConss.py Test file validating the local constraints recipe functionality
src/pyscipopt/scip.pxi Added writeMIP method for outputting MIP relaxations
src/pyscipopt/scip.pxd Added function declaration for SCIPwriteMIP
src/pyscipopt/recipes/nonlinear.py Removed extraneous blank line
CHANGELOG.md Updated changelog with new recipe addition


return [model.getConss(), added_conss]

def getNLocalConss(model: Model, node = None) -> list[int]:
Copy link

Copilot AI Jul 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return type annotation should be -> tuple[int, int] or -> list[int] to be more precise, but the current annotation is consistent with the actual return behavior.

Suggested change
def getNLocalConss(model: Model, node = None) -> list[int]:
def getNLocalConss(model: Model, node = None) -> tuple[int, int]:

Copilot uses AI. Check for mistakes.
@Joao-Dionisio Joao-Dionisio merged commit a22bd67 into master Jul 26, 2025
1 check passed
@Joao-Dionisio Joao-Dionisio deleted the getLocalConss branch August 7, 2025 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants